NetNews Offline 2/NetNews Offline Volume 2.iso/news/comp/lang/c-part1/9305

Text File | 1996-08-05 | 5.3 KB | 174 lines

Path: keats.ugrad.cs.ubc.ca!not-for-mail
From: [e-mail] (Kazimir Kylheku)
Newsgroups: comp.lang.c
Subject: Re: suspicious pointer conversion warning
Date: 8 Mar 1996 15:26:47 -0800
Organization: Computer Science, University of B.C., Vancouver, B.C., Canada
Message-ID: <[e-mail]>
References: <4hpmgt$[e-mail]>
NNTP-Posting-Host: keats.ugrad.cs.ubc.ca

In article <4hpmgt$[e-mail]>,
S. Jon <[e-mail]> wrote:
 >to all the pointer gods out there:
 >
 >i cannot figure out what's wrong with my program.  i always get a
 >suspicious pointer conversion warning when i try to reallocate more memory.
 >
 >the purpose of my program is to open a text file and put all the words
 >into an array.  i initially set my array for 20 words and if there is
 >more than 20 words, i increase the array with a growth factor of 10 by
 >using realloc.
 >
 >i would appreciate any input from anyone.  pointers and i just don't
 >mix!  i can't wait 'til i try linked lists!!!! :)
 >
 >my program is divided into 3 files: wcount.h, wcount.c (main source file),
 >wcount2.c (all my functions).  compiler is borland c++ v3.1 for dos. here is
 >selected parts of my program that i think are relevant:
 >
 >--------------------------------------------
 >//    FILENAME: wcount.h

This is a syntax error. The ISO standard does not define any meaning for a
combination of the // characters outside of a string literal.

Don't mix C++ features into C code.

 >#ifndef WCOUNT_H
 >#define WCOUNT_H
 >
 >typedef struct
 >{
 >  int    iFrequency;
 >  char    szWord[1];   // variable length array
 >}  WordInfo;

There is no such thing as variable length arrays in C. The above is an array of
one character.  Your mastery of C is somewhat lagging behind your enthusiasm to
use what seems to be a variant of Hungarian notation. :)

 >//  FILENAME wcount.c

Where is the inclusion of "wcount.h"?

 >int main ()
 >{
 >  int       iWordCount;
 >  WordInfo  *apWords[20];  // array of pointers to structure WordInfo
 >  FILE      *fpFile;
 >
 >//  program opens file
 >
 >  iWordCount = Parse (fpFile, apWords);
 >
 >//  program displays the words
 >
 >  return (0);
 >}
 >
 >----------------------------------------
 >
 >//  FILENAME wcount2.c

Where is the inclusion of wcount.h? You are using data types here that were
declared in there.

 >int Parse (FILE *fpFile, WordInfo *apWords[])
 >{
 >  int    iWordLimit = 20;  // initially limited to 20 words

Why are you using the magic number 20 here and in main()? How does this
function know that apWords is an array of 20 pointers? At least it could be a
global variable or pre-processor constant.

 >  int    iWordCount;   // running count on the number of words in array
 >
 >//    program parses words here and puts word in array with malloc
 >//    program also compares any new word with words in array to avoid
 >//    duplication


You did not initialize iWordCount (boy, that's hard to read and type!). Its
value is undefined here.

 >  if (iWordCount == iWordLimit)
 >  {
 >    iWordLimit += 10;

At this point you have not read anything from a data file, so why perform a
check? Just grab the memory. You are using an unitialized variable, hence
calling for behavior that is undefined. Doesn't your compiler produce a
diagnostic?

 >//  ***** SUSPICIOUS POINTER ERROR REFERS TO LINE BELOW
 >
 >    apWords = GetMoreMem (apWords, iWordLimit);

You cannot reallocate apWords! The pointer you are trying to reallocate was not
created with malloc(). It is the pointer to  the first element of an array
declared as an automatic variable inside main().

This is completely wrong.

My recommendations:

1.     change the apWords array declaration in main() into a
    pointer rather than an array.

3.    Inside Parse, explicitly malloc() 20 (or however many) pointers, then
    realloc in the given increment when you fill up that limit.

4.    Produce the final array of pointers as a return value.

5.    Return the size by passing the address of an integer into Parse.

Then, inside main() you could write:

    int iWordCount;
    WordInfo **apWords = Parse(fpFile, &iWordCount);

and in Parse() you have something like:

WordInfo **Parse(FILE *fpFile, int *size)
{
    int arraysize = BASE_SIZE;    /* allocated array size    */
    int datasize = 0;        /* actual size        */

    Wordinfo **words = GetMoreMem(0, BASE_SIZE);    /* see recommendation
                               below */

    /* scan the file, reallocating ``words'' as needed */

    *size = datasize;
    return words;
}

 >WordInfo *GetMoreMem (const WordInfo *apWords[], int iWordLimit)
 >{
 >  WordInfo *pTemp;    // temp pointer to struct WordInfo
 >
 >  pTemp = (WordInfo *)realloc(apWords, iWordLimit*sizeof(WordInfo));

This is wrong. You want to allocate a block of pointers. Here you are
allocating a block of WordInfo structures, and assigning it to a pointer to a
WordInfo structure. The corrected code should read:

    WordInfo **pTemp = realloc(apWords, iWordLimit*sizeof(WordInfo *));


 >  if (pTemp == NULL)
 >  {
 >    exit (EXIT_FAILURE);
 >  }
 >  return (pTemp);
 >}

Why don't you check for a null input value for apWords here? If it is null, you
can malloc() a fresh new array. If not, realloc() the existing one. This way,
you can use GetMoreMem() to create the initial array of 20 blocks by starting
with a null pointer (as I did above), and can use the same function to increase
the size later.
--